-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix KeyError when calling pyblish_qml.show() more than once #365
base: master
Are you sure you want to change the base?
Conversation
Code sample to replicate the issue:
|
Thanks for this @jboisvertrodeofx, and for hunting down the original cause. And for the replicable. But this doesn't seem like the right way to go. :/ I think you've identified the actual cause, Install was initially meant to be called by the user. It's designed as a one-off. It got rather consistent that you always call this whenever you show the GUI, so we made it automatic. But now that automation has become a problem, and the solution should be to to silence the warnings. This isn't what you signed up for, but if you are able, I would much rather we revert that commit you found, for Let me know what you think. |
Hey @mottosso thanks for the quick feedback! Just to see if I understanding correctly, you'd add a call to If that's the case, I think it could somewhat work, but one small issue I would see is that we'd have to add a check to see if the callbacks were previously installed before re-installing them, because |
Yes, I think so.
Along with..
And finally..
That way, it'll be guaranteed to be installed whenever the GUI is visible, and most likely be uninstalled whenever it's closed. The exception being crashes or such, when the close event is lost. But that's a fine compromise, as it would be uninstalled the next time around, and ultimately is just an optimisation that won't harm the overall function of the DCC. |
I just committed a WIP, so it will be easier to explain. There seems to be a side effect to reverting the e4dacaa commit, where I was planning on re-using |
It seems that since pyblish-qml-1.9.6, a KeyError is raised when calling
pyblish_qml.show()
more than once ifpyblish.api.deregister_all_callbacks()
is called before the second call.This is the stack trace:
I believe this is this the commit that exposed the issue: e4dacaa
I've seen in other places of the code that we deal with this case with a simple try/except, so I've the done the same in my fix here.